Skip to content

Conversation

goffrie
Copy link
Contributor

@goffrie goffrie commented Apr 10, 2017

Fixes #41161.
It was a little mistake in #40815.

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -4763,6 +4763,8 @@ impl<'a> Parser<'a> {
err.span_label(sp, &"missing `fn`");
}
return Err(err);
} else if let Err(bang_err) = bang_err {
return Err(bang_err);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that if this code used explicit match, mistake wouldn't have been made. Something like

 match (err, bang_err) {
    (Err(_), Err(_)) => ...
    (Err(_), _) => ...
    ...
}

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 14, 2017
@pnkfelix
Copy link
Contributor

While I agree @nagisa has a good suggestion, I also think this change is simple enough and the ICE is problematic enough that I'd rather just land this as is.

@nagisa
Copy link
Member

nagisa commented Apr 14, 2017

Yeah, that’s why I didn’t do a “red cross, can’t land this”-sort of review and a commented instead.

@pnkfelix
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 14, 2017

📌 Commit 1e19647 has been approved by pnkfelix

@pnkfelix
Copy link
Contributor

@bors rollup

@goffrie
Copy link
Contributor Author

goffrie commented Apr 14, 2017

Wait, I just made the change :p

@nagisa
Copy link
Member

nagisa commented Apr 14, 2017

@bors r-

@bors
Copy link
Collaborator

bors commented Apr 14, 2017

🔑 Insufficient privileges

@pnkfelix
Copy link
Contributor

@bors r-

@arielb1
Copy link
Contributor

arielb1 commented Apr 16, 2017

There's #41282 in queue which also fixes this issue.

@goffrie goffrie closed this Apr 18, 2017
@goffrie goffrie deleted the issue-41161 branch April 18, 2017 00:57
@bors
Copy link
Collaborator

bors commented Apr 18, 2017

☔ The latest upstream changes (presumably #41282) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants